feat: support coroutine functions #346
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (68.75%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #346 +/- ##
===========================================
- Coverage 100.00% 98.64% -1.36%
===========================================
Files 21 22 +1
Lines 2120 2216 +96
===========================================
+ Hits 2120 2186 +66
- Misses 0 30 +30 ☔ View full report in Codecov by Sentry. |
|
Thanks for the quick PR @tlambert03!
But this PR currently assumes that the event loop is asyncio. However Trio and AnyIO are very popular, so I'm wondering how we could support them and really be event-loop agnostic. |
|
Because structured concurrency always needs a task group in order to launch tasks, I think it's better to not support async callbacks in psygnal. One can just use a sync callback, and have the async logic in the user code where the new value is put synchronously in a stream, and a background task gets the values from the stream asynchronously. That way the user can launch this background task in the appropriate task group. |
|
Thanks for your input!
Are you sure? I'm definitely happy to do whatever we can here to make using coroutines in psygnal easier. That could include supporting auto detection of asyncio, trio, or anyio. I don't have a ton of personal experience with async/await, so would happily take advice and implement whatever folks suggest. And if there are conveniences we could add around that pattern, that's fine too |
Add support for other event loops
| @pytest.mark.asyncio | ||
| async def test_slot_types(type_: str, capsys: Any) -> None: | ||
| backend = _async.set_async_backend("asyncio") |
There was a problem hiding this comment.
@davidbrochart I think one of the biggest questions for me is "proper usage". This is what I did to get the tests to work (so far, just for asyncio). But note that I had to allow _async.set_async_backend("asyncio") to be called multiple times within a session... (with the restriction that it be called with the same backend).
An important thing to note there though is that, in set_async_backend, I actually had to let it re-initialize the backend (can't just return the already instantiated backend)... otherwise I get
def _check_closed(self):
if self._closed:
> raise RuntimeError('Event loop is closed')
E RuntimeError: Event loop is closed
on follow-up tests.
Another question I have here is how to test all three backends. Is it possible within a single python session?
There was a problem hiding this comment.
Maybe we should still prevent set_async_backend() from being called multiple times, unless a clear_async_backend() function is called in between (mostly for tests)?
| assert cb.dereference() is not None | ||
|
|
||
| cb.cb((2,)) | ||
| await asyncio.sleep(0.01) |
There was a problem hiding this comment.
similar to your comment above... i suspect this is flaky. Should we have an API to wait until the backend has no items in the queue?
There was a problem hiding this comment.
That's a possibility, but a simpler approach could be to set an event in the mock, and wait for it (with a timeout)?
|
one additional note, I'm skipping the |
CodSpeed Performance ReportMerging #346 will not alter performanceComparing Summary
|
fixes #345